mission: move QGC plan import from mission to mission_raw #1343
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
julianoes
force-pushed
the
pr-mission-import
branch
from
19b5ec8
to
476b764
Compare
JonasVautherin
previously approved these changes
Mar 5, 2021
This moves the method to import QGroundControl .plan JSON mission files from the mission plugin to the mission_raw plugin. The reasoning is as follows: The imported missions did not really fit the Mission API, and I guess that the mission progress was often wrong. When the missions were imported into mission, many items/commands were unsupported and dropped with a warning. Such a warning is not visible in the language wrappers and could lead to surprises. The documentation said that the method would fail if any unsupported item was being imported, however, that was not true anymore in code. Therefore, in my opinion, it makes a lot more sense to implement the import in mission_raw where all/any mission items and commands can be imported. It's should be more complete, without surprises, and therefore safer to use. And in later steps we can also add geofence and rally items to this API. This PR contains the proto change and the implementation in code. It also slightly improves the parsing by moving it to a separate file and adding a bit more unit test coverage.
julianoes
force-pushed
the
pr-mission-import
branch
from
a4f5fdd
to
42c4ed1
Compare
|
@JonasVautherin fixed what you raised, squashed commits and force pushed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves the method to import QGroundControl .plan JSON mission files from the mission plugin to the mission_raw plugin.
The reasoning is as follows:
The imported missions did not really fit the Mission API, and I guess that the mission progress was often wrong. When the missions were imported into mission, many items/commands were unsupported and dropped with a warning. Such a warning is not visible in the language wrappers and could lead to surprises. The documentation said that the method would fail if any unsupported item was being imported, however, that was not true anymore in code.
Therefore, in my opinion, it makes a lot more sense to implement the import in mission_raw where all/any mission items and commands can be imported. It's should be more complete, without surprises, and therefore safer to use.
And in later steps we can also add geofence and rally items to this API.
This PR contains the proto change and the implementation in code. It also slightly improves the parsing by moving it to a separate file and adding a bit more unit test coverage.
Depends on mavlink/MAVSDK-Proto#211.